Handle Error queries on subgraphs #936
Conversation
|
@gndelia Please how can this be tested? I resolved this issue based on the screenshot you shared, I would love to know how to recreate and test. |
Thanks for your contribution! These scenarios are always hard to test. You can always replace the |
|
Conflicts! |
@gndelia I've resolved the conflicts locally and trying to push. Currently getting this lint error while trying to push |
|
@qgatssdev for changes under |
qgatssdev
force-pushed
the
handle-error-queries-on-subgraphs
branch
2 times, most recently
from
d14357c to
b6240e6
Compare
qgatssdev
force-pushed
the
handle-error-queries-on-subgraphs
branch
from
b6240e6 to
6b526b6
Compare
|
@gndelia Fixed! Sorry about the messy history, I ran into a problem 😅 |
webapp/utils/subgraph.ts
Outdated
| (response: GraphResponse<{ _meta: { block: { number: number } } }>) => ( | ||
| checkGraphQLErrors(response), | ||
| 'data' in response && response.data._meta.block.number | ||
| ), |
There was a problem hiding this comment.
This is a bit hard to read. Ok, perhaps this one exactly not, but the other .then for GetBtcWithdrawalsQueryResponse becomes very large. I think here, it could be better to add a regular function.
In addition to that, there's a bit of a redundant check for 'data' in response. We know that if checkGraphQLErrors did not throw, then response must be a successful type. I think we can improve that like this:
Update GraphResponse<T> to something like this:
type SuccessResponse<T> = { data: T }
type ErrorResponse = { errors: { message: string }[] }
type GraphResponse<T> = SuccessResponse<T> | ErrorResponseThen, you can update checkGraphQLErrors to something like this
/**
* Helper function to check for errors in GraphQL responses
* @param response The GraphQL response to check
* @throws Error if the response contains errors
*/
function checkGraphQLErrors<T>(
response: GraphResponse<T>,
): asserts response is SuccessResponse<T> {
// Check if response has errors
if ('errors' in response && response.errors.length > 0) {
// Extract error messages and join them
const errorMessages = response.errors.map(e => e.message).join(', ')
throw new Error(`GraphQL Error: ${errorMessages}`)
}
}You can check the docs for that asserts response is SuccessResponse<T>. It works very similar to a user-defined Type Guard.
If we do that, then we can finally replace the function that I am commenting with
| (response: GraphResponse<{ _meta: { block: { number: number } } }>) => ( | |
| checkGraphQLErrors(response), | |
| 'data' in response && response.data._meta.block.number | |
| ), | |
| .then( | |
| (response: GraphResponse<{ _meta: { block: { number: number } } }>) => { | |
| checkGraphQLErrors(response) | |
| // At this point, the compiler knows that `response` is of type `SuccessResponse` and that `response.data` is valid. No need to check again! | |
| return response.data._meta.block.number | |
| }, | |
| ) |
Let's do that, and do something similar for the other returned functions here.
We're getting closer to an optimal solution!
There was a problem hiding this comment.
@gndelia Thanks for the review! I learnt something new, I've made the changes.
Description
This PR addresses an issue where the application fails to properly handle errors from The Graph API. Even though the API returns a 200 OK HTTP status code, it includes error information in the response body that wasn't being checked.
GraphResponsetype to include error fieldscheckGraphQLErrorshelper function to properly detect and handle GraphQL errorsensureJsonParsedhelper function to handle both string and JSON responses consistentlyRelated issue(s)
Closes #931
Fixes #931
Checklist